feat(internal/gengapic): include API version in gapic metadata#1653
feat(internal/gengapic): include API version in gapic metadata#1653noahdietz merged 13 commits intogoogleapis:mainfrom
Conversation
shollyman
left a comment
shollyman
left a comment
There was a problem hiding this comment.
The early return behavior feels a bit itchy to me, but I realize that's an inherited behavior from the proto API.
Yeah...it's a bit weird...my feeling was that if something was called out of order we either 1. return an error (not doing that yet today, so seemed a bit overkill) or 2. do nothing, emit nothing The latter seemed preferred as it would be obvious in tests and generated output that something is missing. I'm happy to change it back to the "silently inits its parent" behavior. It would just be "lossy" if called out of order b.c not every scope has access to apiVersion information. |
I've got some intentions wrt init and setup of this plugin so that we validate things a bit more strongly on startup. I'll keep this in mind when I revisit that work. |
shollyman
changed the title
| if md, ok := g.metadata.GetServices()[tst.serv.GetName()]; !ok { | ||
| t.Errorf("ClientInit(%s) gapic metadata, expected %s to be present but found %+v", tst.tstName, tst.serv.GetName(), g.metadata.GetServices()) | ||
| } else if tst.serv == servPlain && md.GetApiVersion() != servPlainAPIVersion { | ||
| t.Errorf("ClinitInit(%s) gapic metadata service entry api version, got %q, want %q", tst.tstName, md.GetApiVersion(), servPlainAPIVersion) |
3 participants
Pulls out the
google.api.api_versionservice-level annotation and includes it in thegapic_metadata.jsonservicesentry.Also refactors
gapic_metadatageneration to be a little more picky about when things are called because we don't want to silently initialize things and potentially miss adding information we need i.e. theservice-levelApiVersionannotation. This shouldn't be a big deal though because these are always called in a hierarchical way, as the service and then methods are traversed and once for each transport.Internal bug http://b/452365074